test(kubernetes): add optional Envoy-backed e2e mode - #2259
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
0dbb332 to
8e8d0cc
Compare
| use-envoy-gateway: | ||
| description: "Run the e2e command through an Envoy Gateway GRPCRoute" | ||
| required: false | ||
| type: boolean | ||
| default: false |
There was a problem hiding this comment.
What's the motivation for this? Is it to make sure we can test HA deployments?
There was a problem hiding this comment.
The primary motivation for this PR (and some of the others) was to reduce the surface of #1868 (adding HA support) somewhat. This includes adding Envoy configuration / deployments to provide a stable endpoint for the HA case.
As reviewed, the PR added the option but didn't actually exercise any of the added functionality. Since using Envoy is a separate from an HA deployment, I have updated the PR to add an "envoy-enabled" path to the existing tests. This should mean that we can shake out any issues with the envoy-gateway interaction separately from the HA functionality.
Depending on the load on CI, we can consider removing it again or simplifying our test matrix for k8s once HA support lands.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
8e8d0cc to
9b5c53e
Compare
|
/ok-to-test 9b5c53e |
elezar
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This maintainer-authored PR is project-valid: it adds a scoped Envoy-backed Kubernetes E2E path split from #1868, without expanding into HA replicas or external PostgreSQL coverage. @drew asked about the motivation, and @elezar clarified that the goal is to shake out Envoy/Gateway interaction separately from HA behavior; I checked that context against the workflow and script changes.
Head SHA: 9b5c53e4ba4b7a51c95e570cd00d3ac390942653
Review findings:
- Blocking: Envoy mode can expose the plaintext unauthenticated e2e gateway through a default LoadBalancer Service on existing clusters unless the test proxy is forced local/ClusterIP or gated behind an explicit unsafe opt-in.
- Blocking: Envoy mode should wait for Gateway/GRPCRoute/BackendTrafficPolicy acceptance/programming before starting tests.
- Warning: The new
mise run e2e:kubernetes:envoyworkflow and Envoy resources should be reflected in the relevant agent-maintenance/debugging docs such as.agents/skills/debug-openshell-cluster/SKILL.mdand likely.agents/skills/helm-dev-environment/SKILL.md.
Docs: Fern docs are not required because this is developer/E2E workflow behavior, but agent-facing workflow docs need the companion update above.
E2E: test:e2e should run after the review findings are addressed; HA E2E is not required for this scoped Envoy path.
Next state: gator:in-review
| fi | ||
| if use_envoy_gateway; then | ||
| prepare_envoy_gateway | ||
| helm_values_args+=(--values "${ROOT}/deploy/helm/openshell/ci/values-gateway.yaml") |
There was a problem hiding this comment.
gator-agent
Critical: This layers values-gateway.yaml on top of values-skaffold.yaml, which sets server.disableTls=true and server.auth.allowUnauthenticatedUsers=true. In Envoy mode on an existing cloud cluster, Envoy Gateway creates a proxy Service that defaults to LoadBalancer, so the test can expose an unauthenticated plaintext OpenShell gateway beyond the local port-forward. Please either force the test Envoy proxy Service to ClusterIP, or refuse Envoy mode for non-local contexts unless an explicit unsafe opt-in is set.
| exit 1 | ||
| fi | ||
| configure_envoy_backend_policy | ||
| start_gateway_portforward |
There was a problem hiding this comment.
gator-agent
Warning: start_gateway_portforward waits for the Envoy proxy Service and pod readiness, but not for the Gateway, GRPCRoute, or BackendTrafficPolicy to be accepted and programmed. A ready Envoy pod can still serve 404s or enforce default stream/request timeouts while reconciliation catches up. Add an Envoy-mode wait before starting tests that checks Gateway accepted/programmed, GRPCRoute parent accepted/resolved refs, and BackendTrafficPolicy acceptance when created.
|
xref #2469 |
|
@elezar in support of #2469, I recommend refactoring this PR to support multiple ingress implementations or adding a follow-on PR to implement a controller-neutral approach (assign me if needed):
The agentgateway adapter could then install or reuse its controller, create the shared Gateway fixture, configure the chart with |
Summary
Add an optional Envoy-backed execution mode for the standard single-replica Kubernetes E2E suite. The mode exercises the Gateway API/Envoy
GRPCRoutepath without enabling HA, changing replica counts, or requiring PostgreSQL.Direct Service port-forwarding remains the default. Envoy mode installs or reuses an Envoy Gateway controller, creates test-owned Gateway API resources, disables Envoy request and stream duration limits for OpenShell's long-lived gRPC streams, discovers the generated proxy Service, and routes the client port-forward through that Service.
Related Issue
Extracted from the generally applicable Envoy/Gateway API setup in #1868. This PR intentionally leaves HA gateway rebalancing, multi-replica behavior, and external PostgreSQL coverage out of scope while the HA tests are not yet functional.
Changes
mise run e2e:kubernetes:envoyfor local single-replica Envoy execution.OPENSHELL_E2E_KUBE_USE_ENVOY=1support to the Kubernetes e2e wrapper.use-envoy-gatewayinput to the reusable Kubernetes E2E workflow.topology: envoyanduse-envoy-gateway: true.helm installonly when no controller is present, using a unique test-owned namespace.GatewayClassandBackendTrafficPolicyresources, refusing to modify pre-existing resources with those names.Testing
bash -n e2e/with-kube-gateway.shgit diff --checkuv/PyYAMLci/values-gateway.yamlmise run pre-commitcompletes (lint, format, workspace/E2E Clippy, Helm, Python, and other reported checks passed; the first run hit a fresh-virtualenvgrpc_toolsbootstrap race, and the rerun reached the five-minute command timeout during full Rust tests)Checklist